fix(agent): stop apply_patch truncating large files and half-applying patches - #3
Merged
voidstackloop merged 1 commit intoJul 25, 2026
Conversation
… patches Two silent defects in applyPatch: - It used readFile() to load the base content it then wrote back. readFile is display-oriented: it caps at MAX_READ_CHARS (100k) and appends a "[truncated - file is N characters]" marker. Patching any file past that cap therefore destroyed everything after it and saved the marker into the file. Reproduced with a 100k+ file: one one-line hunk, ~50k of source gone. - Files were written as the loop went, so a patch whose later file failed to align left the earlier files already modified. The parser deliberately refuses to fuzzy-match so it can fail loudly rather than misapply; a half-applied patch defeats that. Every file's outcome is now resolved in memory before anything is written. Resolution is staged through a map rather than read straight from disk, so a diff with two sections for one path — or one that creates a file and then patches it — still chains, which a plain read-all-then-write-all would have broken. Note the write phase is still not transactional; this narrows the failure window to a write error rather than eliminating it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two silent defects in
applyPatch, both of which destroy user data without an error.1. Files larger than 100 KB are truncated.
applyPatchloaded the base content it was about to write back withreadFile(). That function is display-oriented: it caps its result atMAX_READ_CHARS(100,000) and appends a[truncated — file is N characters]marker. Patching any file past that cap therefore wrote back the truncated copy — destroying everything after the cap and saving the marker text into the source file.Reproduced with a ~110 KB file and a one-line hunk: the file came back ~100 KB with the truncation marker embedded in it.
rollbackLastWritedoes snapshot the raw previous content, but only for the last 20 writes, in memory, for the current session.replaceInFilealready reads withfs.readFileSyncdirectly, so this was an inconsistency rather than a codebase-wide assumption.2. A patch that fails partway leaves the earlier files already modified.
Files were written as the loop went. A patch whose third file didn't align threw after files one and two were on disk. The parser's own comment says it refuses to fuzzy-match specifically so it can fail loudly rather than misapply — a half-applied patch defeats that.
Fix
readFileForEdithelper that keepsreadFile's "this is a directory" guard but not its display cap. Kept as a separate function rather than a flag onreadFile, so what we show the model and what is on disk don't get conflated again.Resolution is staged through a map rather than each file being read straight from disk. That detail matters: a naive resolve-all-then-write-all breaks two cases that work on
main— a diff with two sections for the same path (the second would read the stale original and silently discard the first edit), and a diff that creates a file and then patches it (the read would fail with ENOENT).git diffdoesn't emit either shape, but a model-authored patch can, and both regressions would have been silent. Tests cover both.Scope note
The write phase is still not transactional — a filesystem error partway through the writes can still leave a partial result. This narrows the window from "any hunk that doesn't align" to "an actual write failure"; it does not eliminate it. Happy to say so in a comment if you'd prefer that recorded in the source.
Tests
Control experiment: the truncation and partial-write tests fail on
mainand pass with the patch; the two chaining tests fail against a resolve-all-then-write-all implementation and pass with the staged one.npx tsc -p tsconfig.json --noEmitclean;npm test231 passed.